Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix registry auth comparison for explicit port numbers #2598

Merged
merged 1 commit into from Jan 31, 2017
Merged

Fix registry auth comparison for explicit port numbers #2598

merged 1 commit into from Jan 31, 2017

Conversation

itaylor
Copy link
Contributor

@itaylor itaylor commented Jan 31, 2017 •
edited

Summary
I use a private NPM registry hosted via Artifactory. When using Yarn, I'm unable to install my modules when it gets to the "Fetching packages" phase, but it gets through the resolving packages phase using the private registry just fine. I traced it back to the npm-registry.js doing a comparison to see if the request string requestUrl.startsWith(registry) this comparison fails on my server because the request url looks like
https://my.server:443/path/to/registry/-/something/somefile.tgz
and the registry url looks like:
https://my.server/path/to/registry/

These are clearly requests to the same url repo, but a string.startsWith doesn't understand that. As a result, the requests are made without auth info and fail with a Not authorized error. Similar errors would occur with the string.startsWith on differing capitalization.

Test plan

I added tests for the function that does the registry comparisons in the __tests__/registries folder. The tests cover both positive and negative cases for the new comparison function.

Run before change:
With .npmrc content in the folder:

registry = https://myserver.mydomain.com/artifactory/api/npm/npm-mkto
_auth = myAuthStringSomeCrazyThing=
always-auth = true

$ yarn install
yarn install v0.19.1
info No lockfile found.
[1/4] Resolving packages...
[2/4] Fetching packages...
error An unexpected error occurred: "https://myserver.mydomain.com:443/artifactory/api/npm/npm-mkto/react-height/-/react-height-2.2.0.tgz: Request failed \"401 Unauthorized\"".
info If you think this is a bug, please open a bug report with the information provided in "/usr/src/app/yarn-error.log".
info Visit https://yarnpkg.com/en/docs/cli/install for documentation about this command.

Run in the same folder with this PR:

$ yarn install
yarn install v0.21.0-0
info No lockfile found.
[1/4] Resolving packages...
[2/4] Fetching packages...
[3/4] Linking dependencies...
[4/4] Building fresh packages...
success Saved lockfile.
Done in 115.13s.

@itaylor
Copy link
Contributor Author

itaylor commented Jan 31, 2017

The Travis-CI failure appears to be unrelated to my changes and doesn't happen when I run the tests locally. It's about a missing file while running a yarn custom script.

(node:3623) UnhandledPromiseRejectionWarning: Unhandled promise rejection (rejection id: 5): Error: Command failed: node /Users/travis/build/yarnpkg/yarn/bin/yarn.js run custom-script
fs.js:1089
  return binding.unlink(pathModule._makeLong(path));
                 ^
Error: ENOENT: no such file or directory, unlink '/Users/travis/Library/Caches/Yarn/.roadrunner.json'
    at Error (native)
    at Object.fs.unlinkSync (fs.js:1089:18)
    at /Users/travis/build/yarnpkg/yarn/node_modules/roadrunner/index.js:52:35
    at Object.reset (/Users/travis/build/yarnpkg/yarn/node_modules/roadrunner/index.js:12:12)
    at Object.<anonymous> (/Users/travis/build/yarnpkg/yarn/bin/yarn.js:41:14)
    at Module._compile (module.js:570:32)
    at Object.Module._extensions..js (module.js:579:10)
    at Module.load (module.js:487:32)
    at tryModuleLoad (module.js:446:12)
    at Function.Module._load (module.js:438:3)
 FAIL  __tests__/index.js (7.383s)
  ● should run custom script

What's the right way to proceed here?

@bestander bestander merged commit bb21491 into yarnpkg:master Jan 31, 2017
@bestander
Copy link
Member

Don't mind travis failing, its MacOS is a bit flaky, I'll add retries for that step

mnsn pushed a commit to mnsn/yarn that referenced this pull request Feb 15, 2017
@itaylor @mnsn
Fix registry auth comparison when registry urls and request urls have…
7303034 
… explicit vs implied port numbers (yarnpkg#2598)
@rogeriochaves rogeriochaves mentioned this pull request Apr 17, 2017
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@itaylor @bestander